Skip to content

[jazzy] Check for nullptrs when pushing new messages to the message cache (backport #2219) #2223

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: jazzy
Choose a base branch
from
Open

[jazzy] Check for nullptrs when pushing new messages to the message cache (backport #2219) #2223

wants to merge 1 commit into from
+32 −3

Conversation

@mergify
Copy link

@mergify mergify bot commented Nov 7, 2025

Description

Adds a simple check to the message cache circular buffer when pushing nullptr messages.
Changed CircularMessageCache to report when dropping messages (null or too large for buffer).

Is this user-facing behavior change?

As it only adds a check if the message is a nullptr, this should not change any normal runtime behavior as this should always trigger a segmentation fault or at minimum UB.

Did you use Generative AI?

No

Additional Information

This is an automatic backport of pull request #2219 done by [Mergify](https://mergify.com).

@jncfa @mergify
Check for nullptrs when pushing new messages to the message cache (#2219
dc70c5e 
)

* fix: add nullptr check when pushing new messages to the message cache

Signed-off-by: Jose Faria <[email protected]>

* refactor: apply changes from review

Signed-off-by: Jose Faria <[email protected]>

* nit: update doxygen comments

Signed-off-by: jncfa <[email protected]>

* Update Doxygen comment for the MessageCacheCircularBuffer::push(msg)

Signed-off-by: Michael Orlov <[email protected]>

---------

Signed-off-by: Jose Faria <[email protected]>
Signed-off-by: jncfa <[email protected]>
Signed-off-by: Michael Orlov <[email protected]>
Co-authored-by: Michael Orlov <[email protected]>
(cherry picked from commit 572f98e)

# Conflicts:
#	rosbag2_cpp/include/rosbag2_cpp/cache/circular_message_cache.hpp
#	rosbag2_cpp/src/rosbag2_cpp/cache/circular_message_cache.cpp
@mergify mergify bot added the conflicts label Nov 7, 2025
@mergify
Copy link
Author

mergify bot commented Nov 7, 2025

Cherry-pick of 572f98e has failed:

On branch mergify/bp/jazzy/pr-2219
Your branch is up to date with 'origin/jazzy'.

You are currently cherry-picking commit 572f98e.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   rosbag2_cpp/include/rosbag2_cpp/cache/message_cache_circular_buffer.hpp
	modified:   rosbag2_cpp/src/rosbag2_cpp/cache/message_cache_circular_buffer.cpp
	modified:   rosbag2_cpp/test/rosbag2_cpp/test_circular_message_cache.cpp

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   rosbag2_cpp/include/rosbag2_cpp/cache/circular_message_cache.hpp
	both modified:   rosbag2_cpp/src/rosbag2_cpp/cache/circular_message_cache.cpp

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@mergify mergify bot mentioned this pull request Nov 7, 2025
Merged
@MichaelOrlov MichaelOrlov changed the title Check for nullptrs when pushing new messages to the message cache (backport #2219) [jazzy] Check for nullptrs when pushing new messages to the message cache (backport #2219) Nov 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

conflicts

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jncfa